Skip to content

Conversation

@a-khushal
Copy link
Contributor

@a-khushal a-khushal commented Jan 19, 2026

Description

Relates to issue: ZEUS-1068
This PR is based on and extends the changes from master...kaloudis:zeus:lnsocket

cc: The LNMessage library now supports React Native. You can view it on my fork here: lnbc1QWFyb24/lnmessage@master...a-khushal:lnmessage:rn-tcp-socket. This fork ensures Commando works with React Native.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

  • Embedded LND
  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (CLNRest)
  • Nostr Wallet Connect
  • LndHub
  • LNSocket

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @a-khushal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant new feature by enabling direct LNSocket connections for Core Lightning (CLN) nodes. This allows users to connect to their CLN instances via a direct TCP connection, enhancing connectivity options beyond existing methods. The implementation involves a new backend service that leverages the 'lnmessage' library, alongside necessary UI adjustments for configuration and updates to project dependencies.

Highlights

  • LNSocket Backend Integration: A new 'LNSocket.ts' backend has been added to support direct TCP connections to Core Lightning nodes using the 'lnmessage' library.
  • Configuration UI: The 'WalletConfiguration' screen now includes dedicated input fields for LNSocket connections, allowing users to specify the node's public key, host, and rune.
  • Dependency Updates: The 'package.json' and 'yarn.lock' files have been updated to include the 'lnmessage' library and adjust 'react-native-tcp-socket' and other related dependencies.
  • Core Lightning RPC Mapping: The new 'LNSocket' backend maps various application-level RPC calls (e.g., 'getTransactions', 'getChannels', 'sendCoins') to their corresponding 'lnmessage' commands.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for direct Core Lightning TCP connections via LNSocket, which is a great new feature. The implementation adds a new LNSocket.ts backend and integrates it throughout the application. However, there are several critical issues that need to be addressed before merging. Most importantly, there's a hardcoded private key in the new backend, and both package.json and yarn.lock contain merge conflict markers. Additionally, I've found a few bugs, including a typo in an RPC parameter and potential crashes due to unsafe data access. I've provided detailed comments and suggestions to fix these issues.

Comment on lines +47 to +46
privateKey:
'31e9739c0d6a2eba36168dd364841cc01d3f4fde221d256e6a781a4dd46715ea',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Hardcoding a private key is a significant security risk. Even if it's a default key for development, it should not be committed to the repository. This key should be generated and stored securely, or at least be configurable and not a hardcoded constant in the source code. Please remove this and implement a secure way to handle the private key.

package.json Outdated
Comment on lines 144 to 150
<<<<<<< HEAD
"react-native-system-navigation-bar": "2.8.0",
"react-native-tcp-socket": "6.2.0",
=======
"react-native-system-navigation-bar": "2.6.4",
"react-native-tcp-socket": "^6.3.0",
>>>>>>> 03f111062 (feat: add LNSocket support for direct Core Lightning TCP connections via lnmessage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This file contains multiple merge conflict markers (<<<<<<<, =======, >>>>>>>). These must be resolved before the pull request can be merged. Please rebase your branch on top of the target branch or resolve the conflicts manually.

yarn.lock Outdated
Comment on lines 3913 to 3920
<<<<<<< HEAD
baseline-browser-mapping@^2.9.0:
version "2.9.11"
resolved "https://registry.yarnpkg.com/baseline-browser-mapping/-/baseline-browser-mapping-2.9.11.tgz#53724708c8db5f97206517ecfe362dbe5181deea"
integrity sha512-Sg0xJUNDU1sJNGdfGWhVHX0kkZ+HWcvmVymJbj6NSgZZmW/8S9Y2HQ5euytnIgakgxN6papOAWiwDo1ctFDcoQ==

=======
>>>>>>> 03f111062 (feat: add LNSocket support for direct Core Lightning TCP connections via lnmessage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This file contains merge conflict markers. This indicates an unresolved merge conflict. Please resolve the conflicts, for example by running yarn install after resolving conflicts in package.json, and commit the updated yarn.lock file.

}));
sendCoins = (data: TransactionRequest) =>
this.rpc('withdraw', {
desination: data.addr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a typo in the property name desination. It should be destination. This will likely cause the 'withdraw' RPC call to fail.

Suggested change
desination: data.addr,
destination: data.addr,

Comment on lines 298 to 316
getRoutes = async (urlParams?: Array<string>) => {
const msatoshi = Number(urlParams && urlParams[1]) * 1000;

const res = await this.rpc('getroute', {
id: urlParams && urlParams[0],
msatoshi,
riskfactor: 2
});

const route = res.route[0];

return {
routes: [
{
total_fees: (route[0].msatoshi - msatoshi) / 1000
}
]
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are a couple of issues in the getRoutes method that could lead to a crash:

  1. On line 299, urlParams && urlParams[1] can be undefined, which results in Number(undefined), which is NaN. This will likely cause issues in the getroute RPC call.
  2. On line 307, res.route[0] is accessed without checking if res.route is empty. If the RPC call returns no routes, this will cause a crash.

Please add defensive checks to handle these cases gracefully.

    getRoutes = async (urlParams?: Array<string>) => {
        const msatoshi = Number(urlParams?.[1] || 0) * 1000;
        const id = urlParams?.[0];

        if (!id || !msatoshi) {
            return { routes: [] };
        }

        const res = await this.rpc('getroute', {
            id,
            msatoshi,
            riskfactor: 2
        });

        const route = res.route?.[0];

        if (!route || route.length === 0) {
            return { routes: [] };
        }

        return {
            routes: [
                {
                    total_fees: (route[0].msatoshi - msatoshi) / 1000
                }
            ]
        };
    };

createInvoice = (data: any) =>
this.rpc('invoice', {
description: data.memo,
label: 'zeus.' + Math.random() * 1000000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using Math.random() for generating a unique label is not guaranteed to be unique and can lead to collisions, especially if invoices are created in quick succession. Consider using a more robust method for generating unique IDs, such as a timestamp combined with random characters, or a library like uuid.

Suggested change
label: 'zeus.' + Math.random() * 1000000,
label: 'zeus.' + Date.now(),

Comment on lines 190 to 203
decodePaymentRequest = (urlParams?: Array<string>) =>
this.rpc('decodepay', [urlParams && urlParams[0]]);
payLightningInvoice = (data: any) =>
this.rpc('pay', {
bolt11: data.payment_request,
msatoshi: data.amt ? Number(data.amt * 1000) : undefined
});
closeChannel = (urlParams?: Array<string>) =>
this.rpc('close', {
id: urlParams && urlParams[0],
unilateraltimeout: urlParams && urlParams[1] ? 60 : 0
}).then(() => ({ chan_close: { success: true } }));
getNodeInfo = (urlParams?: Array<string>) =>
this.rpc('listnodes', [urlParams && urlParams[0]]).then(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The methods decodePaymentRequest and getNodeInfo have unsafe access to urlParams. If urlParams is undefined or empty, [urlParams && urlParams[0]] becomes [undefined], which is likely not the intended behavior and could cause RPC errors. It's better to avoid passing any arguments if urlParams[0] is not available.

@a-khushal a-khushal force-pushed the commando-lnsocket branch 2 times, most recently from e3c8563 to 10aa4bc Compare January 19, 2026 05:09
@kaloudis kaloudis added this to the v0.13.0 milestone Jan 20, 2026
"js-lnurl": "0.5.1",
"js-sha256": "0.9.0",
"lodash": "4.17.23",
"lnmessage": "^0.2.7",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: lock the version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a-khushal , Can we add an isConnected method as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants